Skip to content

bpo-32259: Make a TypeError message when unpack non-iterable more specific.#4903

Merged
serhiy-storchaka merged 5 commits into
python:masterfrom
serhiy-storchaka:unpack-error-msg
Dec 26, 2017
Merged

bpo-32259: Make a TypeError message when unpack non-iterable more specific.#4903
serhiy-storchaka merged 5 commits into
python:masterfrom
serhiy-storchaka:unpack-error-msg

Conversation

@serhiy-storchaka

@serhiy-storchaka serhiy-storchaka commented Dec 16, 2017

Copy link
Copy Markdown
Member

Comment thread Python/ceval.c Outdated

assert(v != NULL);

if (v->ob_type->tp_iter == NULL && !PySequence_Check(v)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to move this test to after the !PyErr_Occurred() test a few lines below? That would take the PySequence() call out of the non-error code path.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I don't understand you. The only !PyErr_Occurred() below is the one inside a loop at line 4154. Do you suggest to move this test after the loop? This doesn't make sense to me, since the loop is run if the argument is an iterable, and this test tests if it is not an iterable.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you're right, sorry.

My point was that you've added a test every time an object is unpacked, which happens a lot. I guess the only way around that is to move this test to after the it == NULL test, and change the text of the error if it's a TypeErrror and if the conditions on this line hold. I'm not sure if it's worth that to avoid the PySequence_Check() call, though. Maybe a quick micro-benchmark would say it's nothing to worry about. I'm not in front of a "real" computer to test, though.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense, thanks.

@ericvsmith

Copy link
Copy Markdown
Member

This looks okay to me.

Comment thread Python/ceval.c Outdated
v->ob_type->tp_iter == NULL && !PySequence_Check(v))
{
PyErr_Format(PyExc_TypeError,
"cannot unpack %.200s object",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cannot unpack %.200s object (it is not iterable)? or cannot unpack %.200s object because it is not iterable?

@serhiy-storchaka serhiy-storchaka merged commit 13a6c09 into python:master Dec 26, 2017
@serhiy-storchaka serhiy-storchaka deleted the unpack-error-msg branch December 26, 2017 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type-feature A feature request or enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants